-
Notifications
You must be signed in to change notification settings - Fork 19.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix-11739: apply continuity line trail for effectline #11893
Conversation
src/chart/helper/EffectLine.js
Outdated
@@ -177,6 +177,7 @@ effectLineProto.updateSymbolPosition = function (symbol) { | |||
var cp1 = symbol.__cp1; | |||
var t = symbol.__t; | |||
var pos = symbol.position; | |||
var lastPos = Array.from(pos); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use Array.prototype.slice.call(pos)
instead of Array.from
because it's not compatible in IE.
Or it can be simply var lastPos = [pos[0], pos[1]];
src/chart/helper/EffectLine.js
Outdated
} | ||
} | ||
else { | ||
symbol.attr('scale', [symbol.scale[0], symbol.scale[0]]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps you should save the original scale
and restore it here. Using scale[0] for both x and y scale will narrow the scenario.
src/chart/helper/EffectLine.js
Outdated
if (this._symbolType === 'line' || this._symbolType === 'rect' || this._symbolType === 'roundRect') { | ||
if ( | ||
lastPos[0] !== 0 && lastPos[1] !== 0 | ||
&& lastPos[0] !== p2[0] && lastPos[1] !== p2[1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the logic here is to make sure lastPos
and pos is in the same period of animation.
Perhaps it's more robust to check current t
and previous t
, make sure current t
is larger than the previous.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's true, thanks pissang ~
Thanks @alex2wong , I can see there is a remarkable visual improvement. Good job! |
Requested revision is done, please help review the new commit if available @pissang , thanks ~ |
Brief Information
This pull request is in the type of:
What does this PR do?
Apply continuity line trail for effectline, currently enable for 'line', 'rect', 'roundRect' symbolType
Fixed issues
#11739
#9771
#8059
Details
Before: What was the problem?
Effect line breaks when zooming in or at low-end device, since symbol is rendered at discrete position
After: How is it fixed in this PR?
Applying scaled symbol to fill the gap between last position and current one
Usage
Are there any API changes?
only enable continuity trail for 'line', 'rect', 'roundRect' symbolType
Related test cases or examples to use the new APIs
test/geo-lines.html
Others
Merging options
Other information